Skip to content

Fix DataFrame column clone behavior - #7754

Open
svick wants to merge 1 commit into
svick/clarify-dataframe-filter-docsfrom
svick/fix-dataframe-clone-behavior
Open

svick wants to merge 1 commit into
svick/clarify-dataframe-filter-docsfrom
svick/fix-dataframe-clone-behavior

Conversation

@svick

@svick svick commented Oct 1, 2026 •

Copy link
Copy Markdown
Member
  • append default values when VBufferDataFrameColumn clone overloads request appended entries
    • doing nothing (previous behavior) or throwing an exception for non-zero numberOfNullsToAppend seemed like worse options
  • allow enumerable index maps to produce more values than the source column
  • add regression coverage for both behaviors

Addresses the two findings from the review on #7750.

Honor appended values for VBuffer clones and allow enumerable index maps to produce more values than the source column.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@svick
svick added this pull request to stack #7755 October 1, 2026 15:46
@svick
svick requested a balanced review from Copilot October 1, 2026 15:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The implementations match the documented behavior and include focused regression coverage.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes cloning behavior for default-value appends and enumerable index maps.

Changes:

  • Appends default VBuffer<T> values during cloning.
  • Allows enumerable index maps longer than the source column.
  • Adds regression tests for both behaviors.
File Description
src/​Microsoft.Data.Analysis/​DataFrameColumn.cs Clarifies clone append semantics.
src/​Microsoft.Data.Analysis/​DataFrameColumns/​VBufferDataFrameColumn.cs Implements default-value appends.
src/​Microsoft.Data.Analysis/​PrimitiveDataFrameColumn.cs Removes the source-length limit on enumerable maps.
test/​Microsoft.Data.Analysis.Tests/​VBufferColumnTests.cs Tests appended default vectors.
test/​Microsoft.Data.Analysis.Tests/​PrimitiveDataFrameColumnTests.cs Tests longer enumerable maps.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@svick
svick marked this pull request as ready for review October 2, 2026 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants